Skip to content

For x86/x64, call a stack probe helper for all frame sizes equal or over one page - #44664

Merged
echesakov merged 1 commit into
dotnet:masterfrom
echesakov:Jit-Always-Use-Stack-Probe-Helper
Nov 19, 2020
Merged

For x86/x64, call a stack probe helper for all frame sizes equal or over one page#44664
echesakov merged 1 commit into
dotnet:masterfrom
echesakov:Jit-Always-Use-Stack-Probe-Helper

Conversation

@echesakov

@echesakov echesakov commented Nov 13, 2020

Copy link
Copy Markdown
Contributor

This is to avoid accessing stack below rsp in a prolog. This is how the probing is done in MSVC++ implementation for frames larger than PAGE_SIZE.

@echesakov echesakov added arch-x86 os-linux Linux OS (any supported distro) os-windows arch-x64 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Nov 13, 2020
@echesakov echesakov self-assigned this Nov 13, 2020
…lper for larger frame sizes in codegenxarch.cpp
@echesakov
echesakov force-pushed the Jit-Always-Use-Stack-Probe-Helper branch from 4000867 to 15b7556 Compare November 13, 2020 21:42
@echesakov
echesakov marked this pull request as ready for review November 17, 2020 03:09
@echesakov

Copy link
Copy Markdown
Contributor Author

@janvorli @BruceForstall PTAL - I ran dotnet/performance benchmarks on top of these changes.
There is no measurable performance impact - if a benchmark was Slower/Faster during the first run, it was showing the Same results for the second run.

With help from @DrewScoggins we ran the same changes on our performance machines and, although there are some benchmarks that are slower with the change, looking at the historical data I can't reliably whether it's going make it's slower. Please let me know if you want me to share the data with you.

@BruceForstall BruceForstall left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. btw, the title, "Always call a stack probe helper", is a too ambition. Instead, I would say "For x86/x64, call a stack probe helper for all frame sizes equal or over one page." The only difference here is we don't inline probe 2 and 3 page frames, which was an optimization someone added long, long ago (before my time) that has persisted.

@BruceForstall

Copy link
Copy Markdown
Contributor

btw, I think we need a detailed section the in the CLR ABI document, https://github.com/dotnet/runtime/blob/master/docs/design/coreclr/botr/clr-abi.md, describing all the rules and restrictions and design constraints around stack probing on all the different platforms. (Or maybe it deserves its own document.)

@echesakov

Copy link
Copy Markdown
Contributor Author

btw, I think we need a detailed section the in the CLR ABI document, https://github.com/dotnet/runtime/blob/master/docs/design/coreclr/botr/clr-abi.md, describing all the rules and restrictions and design constraints around stack probing on all the different platforms. (Or maybe it deserves its own document.)

Yes, I do remember that I need to update the document (or create a new one as suggested). I am still in the process of deciding what should be done on Arm64. After that, I can go and describe all the implementations.

@janvorli janvorli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, as far as I can tell given my limited knowledge of JIT internals.

@echesakov echesakov changed the title Always call a stack probe helper For x86/x64, call a stack probe helper for all frame sizes equal or over one page Nov 19, 2020
@echesakov
echesakov merged commit 999785e into dotnet:master Nov 19, 2020
@echesakov
echesakov deleted the Jit-Always-Use-Stack-Probe-Helper branch November 19, 2020 01:39
@ghost ghost locked as resolved and limited conversation to collaborators Dec 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-x64 arch-x86 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI os-linux Linux OS (any supported distro) os-windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants